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

feat: allow Args on main run #982

Closed
wants to merge 1 commit into from

Conversation

ajnavarro
Copy link
Contributor

Attempt to implement Args on main() runs.

Problem: I didn't find a clean way to inject os.Args values.

Contributors Checklist

  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

Signed-off-by: Antonio Navarro Perez <antnavper@gmail.com>
@github-actions github-actions bot added 📦 🤖 gnovm Issues or PRs gnovm related 📦 🌐 tendermint v2 Issues or PRs tm2 related labels Jul 20, 2023
@ajnavarro ajnavarro mentioned this pull request Jul 20, 2023
Comment on lines +4 to +10
var Args []string

func init() {
Args = runtime_args()
}

func runtime_args() []string // in package runtime
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried two possible solutions:

  • Directly inject os.Args values when calling RunMain function on the VM. Problem: os package is not available by default on the VM, you need VMKeeper for it.
  • Implementing runtime_args function somehow, returning the provided arguments.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second option.

I suggest: VMKeeper to be the preferred "frontend" for configuring the VM for advanced use cases like full context, multi-transaction features etc, while the raw gno.Machine remains suitable for single-test focused tasks.

Related with #1016.
After #1016 will be finished, I plan to migrate most of the gno CLI to it instead of gno.Machine, especially the REPL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

@ajnavarro
Copy link
Contributor Author

ajnavarro commented Aug 9, 2023

Closing this PR because it was just opened to get some feedback from @moul . More context here: #982 (comment)

@ajnavarro ajnavarro closed this Aug 9, 2023
@moul moul added this to the 🌟 main.gno.land (wanted) milestone Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: Done
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants