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

Interaction between command line arguments and ini file #555

Open
corrados opened this issue Aug 29, 2020 · 20 comments
Open

Interaction between command line arguments and ini file #555

corrados opened this issue Aug 29, 2020 · 20 comments

Comments

@corrados
Copy link
Contributor

For me, the headless Jamulus server on the Linux command line should behave like any other GNU tool. You configure it with command line arguments and that's it. No hidden ini file changes the settings.

The situation is different if I run the client/server with a GUI on, e.g., Windows. For that operation my expectation is that the software stores/recovers my settings I set in the GUI.

pljones and I have a different opinion on that. See the discussion here #370 (comment) and here #370 (comment) and here #370 (comment).

I think we need some more opinions on that matter. Please discuss the expected behaviour in this Issue.

@WolfganP
Copy link

WolfganP commented Aug 29, 2020

My take is similar to the scattered comments around:

  • For GUI users, I believe there's the expectation that an ini/cfg file will store the session settings (be loaded at pgm start time, and stored there if modified)
  • For CLI users, I sense them more technical oriented, and willing to play with documented features and settings priorities. ie, for me:
    - CLI arguments are king and override everything else (hardwired defaults or ini file import)
    - if an ini/cfg file exists, it will be imported to complement (not overwrite) the set of settings for the session. The actions (ini file imported, settings adopted) should be announced on stdout/stderr.
    - in headless mode, there's no interaction with the program (except server's basic recording control) so no expectation of ini file being rewritten/updated.

A complement to certain jamulus/system announcements may be the use of the chat window to push for important notifications (ie server outdated)

@pljones
Copy link
Collaborator

pljones commented Aug 29, 2020

Just to be clear, the scenarios being consider have the following use cases (this describes my understanding of the current intended behaviour):

  • If I run without an ini file, and without a gui, settings should not be written.
    (Pass -e Rock, no inifile updates.)

  • If I run with an ini file, but without a gui, settings should not be written.
    (Load an inifile with Central Server Jazz, pass -e Rock, the inifile stays as Jazz.)

  • If I run without an ini file, but with a gui, settings should be written.
    (Pass -e Rock, default inifile gets updated.)

  • If I run with an ini file, and with a gui, settings should be written.
    (Load an inifile with Central Server Jazz, pass -e Rock, the inifile gets update to Rock.)

The first two cases I have no issue with.

However, to me, regardless of whether or not I'm using a GUI, if I've loaded some settings from an ini file and some settings from the command line, the command line settings should not -- automatically -- overwrite what's in the ini file.

Either those fields in the GUI need to be highlighted and a specific action to confirm the value taken (i.e. manually change to something else or some "confirm" action to remove the highlight) -- or the field would be better blocked from editing.

@corrados
Copy link
Contributor Author

this describes my understanding of the current intended behaviour [...] If I run with an ini file, but without a gui, settings should not be written. (Load an inifile with Central Server Jazz, pass -e Rock, the inifile stays as Jazz.)

No, this is not the intended behaviour. If a settings file is given, the command line arguments always overwrite the settings in the ini file and also the command line parameter is stored in that ini file.

However, to me, regardless of whether or not I'm using a GUI, if I've loaded some settings from an ini file and some settings from the command line, the command line settings should not -- automatically -- overwrite what's in the ini file.

Hm, we had this discussion before. I thought we agreed that we apply the simple rule that command line arguments always overwrite ini settings and that command line arguments are stored in the ini file.

Either those fields in the GUI need to be highlighted and a specific action to confirm the value taken (i.e. manually change to something else or some "confirm" action to remove the highlight) -- or the field would be better blocked from editing.

I would not like such a GUI behaviour. If I specify a value on the command line, I want it to be applied to the GUI directly. That is the intention of the command line argument. No need to confirm this.

@pljones
Copy link
Collaborator

pljones commented Aug 29, 2020

If I run with an ini file, but without a gui, settings should not be written. (Load an inifile with Central Server Jazz, pass -e Rock, the inifile stays as Jazz.)

No, this is not the intended behaviour. If a settings file is given, the command line arguments always overwrite the settings in the ini file and also the command line parameter is stored in that ini file.

That's not what I've written says.

I said: if I pass a command line setting, it overrides any value in any inifile passed (or defaulted).

@pljones
Copy link
Collaborator

pljones commented Aug 29, 2020

If I specify a value on the command line, I want it to be applied to the GUI directly. That is the intention of the command line argument. No need to confirm this.

That's not what I've written says. The value would be visible in the GUI. It would be highlighted, in fact. Quite present, quite visible, quite effective. No need to "confirm" anything to get the behaviour requested from the command line setting.

@corrados
Copy link
Contributor Author

If I run with an ini file, but without a gui, settings should not be written. (Load an inifile with Central Server Jazz, pass -e Rock, the inifile stays as Jazz.)

No, this is not the intended behaviour. If a settings file is given, the command line arguments always overwrite the settings in the ini file and also the command line parameter is stored in that ini file.

That's not what I've written says.
I said: if I pass a command line setting, it overrides any value in any inifile passed (or defaulted).

What I wanted to say in my comment is: You write: "If I run with an ini file [...] settings should not be written" -> If a settings file is given, the current settings should be stored in that file. But you write that it should not.

@corrados
Copy link
Contributor Author

If I specify a value on the command line, I want it to be applied to the GUI directly. That is the intention of the command line argument. No need to confirm this.

That's not what I've written says. The value would be visible in the GUI. It would be highlighted, in fact. Quite present, quite visible, quite effective. No need to "confirm" anything to get the behaviour requested from the command line setting.

I was referring to your comment: "some "confirm" action to remove the highlight". Ok, I think I misunderstood that sentence. You say to remove the highlight but I interpreted it that the parameter is only applied when confirmed.

Anyway, I do not see a need for a highlight. I just makes the code more complex with little benefit. Do you know of any user who complained that a command line setting has overwritten his inifile setting?

@WolfganP
Copy link

I think the agreement is that CLI arguments have the highest precedence. That should be clearly stated in the wiki's reference page for advanced users to take into account (https://github.com/corrados/jamulus/wiki/Command-Line-Options makes no mention of priorities/precedence as of now).

I don't see the need to complicate the code and highlight conflicting values in GUI mode, as we should assume the user who customized the command line invocation has some technical knowledge and read the wiki to know cause and effect of his/her customization (or at least know that messing with configs around w/o understanding is not a good thing :-) )

@pljones
Copy link
Collaborator

pljones commented Aug 30, 2020

Based on Volker's clarifications:

  • If I run without an ini file, and without a gui, settings should not be written.
    (Pass -e Rock, no inifile updates.)

  • If I run with an ini file, but without a gui, settings should be written.
    (Load an inifile with Central Server Default, pass -e Rock, the inifile gets update to Rock.)

I'll skip the "with gui" ones - basically, we write back always for them. It's here I'll highlight a concern. Would someone running headless using a command line that specified an inifile expect that file to get updated from a command line parameter? I'd say not.

Alternatively, headless should work the same as with the gui -- use the default inifile or the specified inifile and write back. (Essentially, this is the change I'd made in the other thread.)

Not some inconsistent half-way house, sometimes writing back and sometimes not.

Maybe I use the GUI to edit the welcome message. I don't specify an inifile, so the default inifile is edited. Now I run my server headless. Does it pick up my message? Hm, no. Oh, I suddenly must supply the inifile name or it's ignored. OK. I pass the inifile. But my server doesn't get registered as default central is full. Fine, I'll try Rock. No, that's no good as I'm not running that specific a server. I'll drop the override Rock. But now it keeps trying to use Rock. Weird. Even weirder if the user had tried switching before specifying the inifile as they'll have not had the same behaviour.

@pljones
Copy link
Collaborator

pljones commented Aug 30, 2020

Let's extend the above. To use the GUI, I run a remote X session. I can't do that from my mobile phone too well but I've got ssh okay.

So I've worked on my welcome message. It's quite extensive. Shows various bits of useful information on how best to get things going and so forth. I'm happy with it and start up the server.

A couple of days later, reading Facebook from my phone, I find I've put the wrong url for a link and need to stop it showing - but I'm away from my computer. Still, I login via ssh and I quickly append -w '' to the command line settings. Simple.

Back at my computer, I just want to correct that URL and let the message display again.

But based on the above, that -w '' was destructive -- it deleted the entire welcome message from the inifile! I have to start all over again, my creative juices wasted.

@pljones
Copy link
Collaborator

pljones commented Aug 30, 2020

Another example. An SMTP server. It's headless. Let's say v0.0.1 has no configuration file support. It'll start up, defaults to listening on port 23 and dumps all mail received into a dead letter basket. It takes three command line settings: port, local domain and outbound server. If you pass local domain, it now knows to check the local user names for matches when the host name of the incoming email matches and deliver to that user's mailbox. Everything else to dead letter, still... Outbound domain lets you say where to send emails received from the local domain when the to field isn't in the local domain. Simple mail transport at its best. Port, of course, lets you override the port.

So this is simple. But life isn't. I want subdomains. I've multiple local servers on my network and want to handle that. Suddenly it's all too much for the command line settings. I introduce a config file. Of course, you don't need to use it if your life is simple.

Now, I can pick from a few things:

  • I could require a config file to be specified in a command line setting and fail if not specified
  • Or I could allow a config file to be specified in a command line setting and revert to the "simple world" behaviour if it's not specified
  • Or I can check a default location for a config file and fail if it's not there
  • Or I can check a default location for a config file and revert to the "simple world" behaviour if it's not there

... or ... I can allow a command line setting to specify a config file. If it's not specified, I can check a default location for a config file. And if I've still not got one, revert to the "simple world" behaviour.

That last one seems most flexible. It also seems to me the most common approach used in servers I've seen (although many now are so complex the "simple world" behaviour isn't an option).

OK, so I either have or have not got a config file. I go off and read it. But what if I've got more command line settings?

Well, "obviously" they take priority over the content of the file.

Let's say I've got my live SMTP server running on port 23. I want to run a few tests with pretty much the same config but a different outbound server. Easy - specify the existing live config file and override the outbound server. Hm. And the port, otherwise it'll clash. Sorted.

Right? Would you expect the inifile to be updated? I wouldn't.

@corrados
Copy link
Contributor Author

corrados commented Sep 1, 2020

Well, both of your examples are fine with the current implementation. I just checked: For the current headless server (i.e. nogui) there is no settings support implemented at all. I.e. even the --inifile does not work with the current code. That makes things easy, since no setting file is involved, just command line arguments.

You may argue that the inifile makes sense for extensive welcome messages. But there is a "hidden feature" which we may make more visible to the Jamulus users: https://github.com/corrados/jamulus/blob/master/src/server.cpp#L397. You can use a file instead of the welcome message. E.g.:
./Jamulus -s -n -w /home/myuser/jamuluswelcomemessage

@WolfganP
Copy link

WolfganP commented Sep 1, 2020

You may argue that the inifile makes sense for extensive welcome messages. But there is a "hidden feature" which we may make more visible to the Jamulus users: https://github.com/corrados/jamulus/blob/master/src/server.cpp#L397. You can use a file instead of the welcome message. E.g.: ./Jamulus -s -n -w /home/myuser/jamuluswelcomemessage

Just added to the cli wiki page.

@pljones
Copy link
Collaborator

pljones commented Sep 1, 2020

I said:

Based on Volker's clarifications:

  • If I run without an ini file, and without a gui, settings should not be written.
    (Pass -e Rock, no inifile updates.)
  • If I run with an ini file, but without a gui, settings should be written.
    (Load an inifile with Central Server Default, pass -e Rock, the inifile gets update to Rock.)

Response:

For the current headless server (i.e. nogui) there is no settings support implemented at all. I.e. even the --inifile does not work with the current code.

OK, so what you're saying is that it doesn't work the way you'd previously said it worked now you come to try it out.

I'd have said that means it's confusing even to you -- or it's been broken at some point (possibly unintentionally when the HEADLESS support went in).

But in that case, it does become consistent. We get: with GUI, uses inifile always; without does not use inifile ever. That's simple enough.

@pljones
Copy link
Collaborator

pljones commented Sep 13, 2020

I'm happy for this to be closed now the help has been updated.

@ann0see
Copy link
Member

ann0see commented May 6, 2022

Re opening to allow discussion for @pgScorpio

@ann0see ann0see reopened this May 6, 2022
@pgScorpio
Copy link
Contributor

Re opening to allow discussion for @pgScorpio

Thanks @ann0see, I didn't notice this issue until now. (Since this subject is discussed in multiple places we maybe better start a real discussion on Settings and ini file handling?)

As I mentioned before in other issues/PR's I'm also working on a new implementation for settings and commandline parameter handling. And, as I see It, there would be no propper reason why a headless build should not be able to use an ini file. (Maybe only if an --inifile parameter is given ?)

Regarding commandline parameters overwriting ini file settings I think a commandline parameter should overide the ini setting not overwrite. So if a commandline parameter is given that value will be used for that session, but the value written to the ini file will not be changed, so when running the next time without the commandline parameter the original ini file value will be used again.
When running in GUI mode and a commandline parameter is given the commandline parameter will also override the ini file value, until the value is changed via the GUI. In that case the ini file value will be changed and the commandline parameter no longer overrides.

The complete Settings/Commandline parameter handling overhaul I'm working on would already allow this, and it also allows to store commandline parameters (See also here ), even those that who's values are normally not stored in the ini file. (ipv6, clientname, etc...) These stored commandline parameters will behave as if given on the commandline, so also not changing any normal ini file values.
The new implementation will also allow to have a sectioned ini file that could contain client settings as wel as server settings and also could have separate sections under "client" and "server" for i.e. user profile, fader settings and audio device settings, making the ini file much more structured.

@pgScorpio
Copy link
Contributor

@pljones

https://github.com/pljones/jamulus/tree/feature/next-big-thing-for-settings/src https://github.com/pljones/jamulus/blob/feature/next-big-thing-for-settings/src/options.h https://github.com/pljones/jamulus/blob/feature/next-big-thing-for-settings/src/options.cpp

I'm aware of your work on settings and commandline options, and I already asked you multiple times to work on this together (until now, unfortunately without any reply).
I'm afraid you're still going in the wrong direction... Did you already have a look at my solution ?
cmdlnoptions.h
cmdlnoptions.cpp
settings.h
settings.cpp
Though it's a similar approach, I think it's more straight forward, more versatile, faster and easier to maintain.

I would also appreciate the opinions of others on these two approaches.

@pljones
Copy link
Collaborator

pljones commented Jun 14, 2023

There's two main things I don't like:

  • generic validation of the values doesn't catch command line syntax errors soon enough (unless I'm missing something)
  • non-integrated command line and settings - I mean think each setting should have a command line override and each command line option should also have persistent state

My "options" approach deals with both of those. Each option provides its own validation (based on some "built in" methods and allowing where something really special should happen, e.g. --ctrlmidich). Each option will also get stored in any inifile, with the command line always overriding for that execution (i.e. not persisting -- though I think I hadn't quite finished that when I had to stop work).

Admittedly, it's ghastly complicated and pretty horrible code currently, and I tried to include splitting the GUI and Client/Server code more cleanly at the same time, meaning a pretty much total rewrite... Which rather stopped me when too many changes to Jamulus were made for my rebases to be clean...

@ann0see ann0see added this to Tracking Jul 1, 2023
@github-project-automation github-project-automation bot moved this to Triage in Tracking Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triage
Development

No branches or pull requests

5 participants