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

Pony RT options passing #155

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Pony RT options passing #155

wants to merge 1 commit into from

Conversation

alexsnaps
Copy link

@alexsnaps alexsnaps commented Nov 15, 2019

Pony runtime tweaks through arguments

The existing

  • --pony* arguments are provided to the ponyc outputted executable
  • We’ve enhanced bad input handling to existing arguments (required option value existed, now we also check for boundaries)
  • Added --ponyhelp as the input grew
  • Existing recognized --pony* arguments are not exposed through Env.args (which I think is a wise decision, I only see pros to it, but in case some think this might be revisited as part of this, I guess it could)

Issues with the existing

  • When renaming --ponythreads to --ponymaxthreads , an upgrading user could miss this change, and this would just be silently ignored. In this very case resulting in ignoring the user specified value and falling back to default. Also resulting in the old argument now being exposed to his Env.args
  • Once GA (i.e. post release 1.0) how can we deprecate arguments? How can we even add one, without possibly break existing code (e.g. someone using that arg already)?

This RFC tries to address the above. Mostly trying to get the ball rolling again. I've had some personal things going on that came in between me and this… 

Moving forward I'll start implementing and filling the Detailed design section in. But wanted to get this discussion going again… in a more "formal" way than on Tulip. So please have at it! 🙇

Formatted version here

@alexsnaps alexsnaps changed the title Added 0065 on Pony RT options passing Added draft on Pony RT options passing Nov 17, 2019

# Detailed design

Building upon [RFC-0064](0064-llvm-cli-opts-integration.md), as we probably should avoid introducing a new pattern, the proposal is to have a single option, namely `--pony_opts`, to pass all options as a single, comma seperated string, for the runtime to consume, e.g.:
Copy link
Member

Choose a reason for hiding this comment

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

--pony-opts might be more idiomatic for CLI than --pony_opts?

That is, dashes are probably more idiomatic than underscores.

@SeanTAllen SeanTAllen added the status - final comment period The RFC is finalized. Waiting for final comments. label Dec 10, 2019
@SeanTAllen
Copy link
Member

Hi @alexsnaps,

We discussed this at sync. No one has strong feelings about what should replace what we have (in terms of your alternatives) beyond the "-" instead of "_".

We think either of your alternatives is better than what we have so we are good with moving forward with them.

Thank you for this RFC. We have put it into a final comment period and will be voting on it soon. Can you update to change the _ to -; assuming you agree with that request.

@SeanTAllen SeanTAllen changed the title Added draft on Pony RT options passing Pony RT options passing Dec 10, 2019
@redvers
Copy link

redvers commented Dec 15, 2019

Does it make sense to have a call to provide the software author the ability to:

  1. Potentially disable the options.
  2. Have visibility of the options if they choose? like env.ponyargs?

@SeanTAllen
Copy link
Member

@redvers What options do you imagine the software author would want to disable? What does "disable" mean to you?

Can you give some scenarios you are thinking of?

@redvers
Copy link

redvers commented Dec 16, 2019

  1. has been implemented already by the looks of it.
  2. Visibility of the options would be good for logfiles so we can see in logs if our end-users alter any of the runtime values.

@rhagenson
Copy link
Member

Looks good to me. I agree on the - rather than _ change.

Personally, I think the namespacing alternative is superior to the environment variable route. The Pony philosophy includes Correctness as its first tenet and Consistency as the second to last. Managing environments in addition to executables is a hassle and decreases the likelihood of correctness, and running without the same environment would break consistency. A given program should run correctly and consistently wherever one chooses to run it.

My thought on a sort-of halfway to using environment variables (where one gets to always use their preferred defaults) is to bake the desired runtime defaults into the executable via compile-time options. People would get to set their preferred defaults when they compile the program, but it executes with the same semantic defaults no matter where it is run despite the environment. Configuring these personal preferences might be a good feature for the dependency manager corral since it would allow compiling for different semantic environments such as prod and dev (and perhaps one day distributed, where environments would be a royal mess to manage).

@jemc
Copy link
Member

jemc commented Apr 28, 2020

Removing from final comment period until the - vs _ point is addressed in some way.

@jemc jemc removed the status - final comment period The RFC is finalized. Waiting for final comments. label Apr 28, 2020
Base automatically changed from master to main February 8, 2021 22:15
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

Successfully merging this pull request may close these issues.

5 participants