Add possibility to reference environment variables in config (BZ#5632)

[qx-bug-importer]

Tristan Koch (@trkoch) wrote:

For example, a configuration file could look like this:

"let" : { "FOO": "$${FOO}" }

(Note the double $).

Provided 'echo $FOO' returns 'foo', the macro should evaluate to 'foo'.

One use case I see for this enhancement is to define the (full qualified) path to qooxdoo system-wide. This would allow to move qooxdoo projects without the need to adjust the configuration or overwrite the macro on invocation.

assigned to Thomas Herchenroeder (@thron7)

[qx-bug-importer]

Thomas Herchenroeder (@thron7) wrote:

I've made a first implementation, adding the OS enviroment keys to the default macros. This doesn't yet fix the QOOXDOO_PATH use case, as currently this macro is needed to include other configs, and this is earlier than the default macros are evaluated.

With 101a31c .

[qx-bug-importer]

Derrell Lipman (@derrell) wrote:

> I've made a first implementation, adding the OS enviroment keys to the default > macros. This doesn't yet fix the QOOXDOO_PATH use case, as currently this macro > is needed to include other configs, and this is earlier than the default macros > are evaluated.

Cool! Can't wait! So glad you're working on this! Not being able to define QOOXDOO_PATH on a per-user basis has been a huge headache, with multiple people working on the same project.

[qx-bug-importer]

Thomas Herchenroeder (@thron7) wrote:

I moved the OS environment settings from the Default to the Config class. This way, they're available earlier, esp. for expansion of the top-level "include" keys (which allows you to specify QOOXDOO_PATH in the OS environ). They are included into the config global "let" section of the config, and take precedence over keys already found there (which I think is intuitive). Also, it seems like a better precedence that jobs will first include those before including macros from parent jobs, so environment can override keys in config files.

Having QOOXDOO_PATH in the environment now seems to work, as well as more job-specific macros.

With bcf93aef54.

[qx-bug-importer]

Thomas Herchenroeder (@thron7) wrote:

NB: The implementation does not exactly match the bug description. Environment keys are "transparent" and appear just like any other macros. So there is no special syntax to refer to them (Which I think is better, as you wouldn't want to commit to how a value is provided when you are writing a reference to it in a config file). Environment settings have high priority. They take precedence over all keys of like names that are written down in config files, and should (not tested!) have lower precedence than macros passed on the command line (with -m/--macro).

[qx-bug-importer]

Derrell Lipman (@derrell) wrote:

> Environment settings have high priority. They take > precedence over all keys of like names that are written down in config files,

I believe this implementation differs from what is typically expected and found. Most typically, in my experience, the environment variable is a default which can be overridden by a configuration file, which can be overridden by a command line option.

Here are some examples of documentation for popular projects which do it the way I just described, rather than the way it was just implemented here:

http://httpd.apache.org/docs/2.3/configuring.html http://gcc.gnu.org/onlinedocs/gcc/Environment-Variables.html

I don't actually know of any examples where the environment takes precedence over a configuration file.

[qx-bug-importer]

Derrell Lipman (@derrell) wrote:

Reopened due to comment 5

[qx-bug-importer]

Thomas Herchenroeder (@thron7) wrote:

Hi Derrell,

> > > Environment settings have high priority. They take > > precedence over all keys of like names that are written down in config files, > > I believe this implementation differs from what is typically expected and found. Most typically, in my experience, the environment variable is a default which can be overridden by a configuration file, which can be overridden by a command line option. > > Here are some examples of documentation for popular projects which do it the way I just described, rather than the way it was just implemented here: > > http://httpd.apache.org/docs/2.3/configuring.html > http://gcc.gnu.org/onlinedocs/gcc/Environment-Variables.html > > I don't actually know of any examples where the environment takes precedence over a configuration file.

good point (although I think there might be valid examples where it is not like that). In the case of qooxdoo I feel things are different. In the examples you gave, the configuration file is all user-driven. The user owns it, writes it, maintains it, etc. Those apps typically can do well without a config file, and deliver just a sparse template configuration when unpacked. All configuration files are exposed (not withstanding user access permissions and such).

In qooxdoo the difference is that a user interacts with, like, 7 config files when just using a gui skeleton, most of them hidden from him as they are part of the framework, and not meant to be directly exposed. Following your logic he would have no chance of overriding any of the macros that come with them, except for sitting down and writing the corresponding stuff in his own config.json. I feel this wouldn't do justice to the user. I think the goal is always to give higher precedence the closer a setting is to immediate user control. And in the case of qooxdoo I think environment settings are "closer" to the user than config files. You have to think of all of them.

[qx-bug-importer]

Derrell Lipman (@derrell) wrote:

Understood. But if I understand what you've implemented correctly, that means that if you want to override the environment's QOOOXDOO_PATH for one of his projects, for example, it can't be done. One would expect that putting a QOOXDOO_PATH setting in the "let" section would override what came from the environment, for that project. In the format for which I provided the references, it's always possible to override a "more broad" definition ("broadest" being environment, then config file, then command line option) with a "more concise" (less broad) one. Is it possible, as you've implemented it, to override an environment variable in the config file without the fairly significant work of rewriting a bunch of sub-jobs?

[qx-bug-importer]

Thomas Herchenroeder (@thron7) wrote:

> Understood. But if I understand what you've implemented correctly, that means > that if you want to override the environment's QOOOXDOO_PATH for one of his > projects, for example, it can't be done.

Why not?! Many teams I know maintain multiple bash profiles for different projects, and do a ". ~/.projectA", ". ~/.projectB", etc. to switch between them.

> One would expect that putting a > QOOXDOO_PATH setting in the "let" section would override what came from the > environment, for that project.

But I thought that was exactly what you were trying to avoid: Changing a config file to customize it for a local environment, as the config file might be in version control!?

> In the format for which I provided the > references, it's always possible to override a "more broad" definition > ("broadest" being environment, then config file, then command line option) with > a "more concise" (less broad) one.

Yes, only that the ordering from "broad" to "less broad" sorts the same items differently for qooxdoo IMO: config file, environment, command line.

> Is it possible, as you've implemented it, to > override an environment variable in the config file without the fairly > significant work of rewriting a bunch of sub-jobs?

As I've implemented it, an environment-based key takes precedence over a file-based key. So, no, you cannot override an environment key in a config file, no matter how far you go. (You can of course re-write existing jobs to not use the key at all, if that is what you are hinting at, but that would be a drag). If you don't like the environment value, you can always unset it, or pass the macro on the command line.

[qx-bug-importer]

Derrell Lipman (@derrell) wrote:

> Why not?! Many teams I know maintain multiple bash profiles for different > projects, and do a ". ~/.projectA", ". ~/.projectB", etc. to switch between > them.

Granted. That's a reasonable solution.

> > > One would expect that putting a > > QOOXDOO_PATH setting in the "let" section would override what came from the > > environment, for that project. > > But I thought that was exactly what you were trying to avoid: Changing a config > file to customize it for a local environment, as the config file might be in > version control!?

Yes that is exactly what I'm trying to avoid, in the general case. My arguments here are for an interface that I would find intuitive because it is the way other programs tend to work. You have already succeeded in accomplishing my overall goal (thank you!), and before this gets too widely used, I wanted to bring up what feels to me quite unintuitive.

> Yes, only that the ordering from "broad" to "less broad" sorts the same items > differently for qooxdoo IMO: config file, environment, command line.

Your brain works differently than mine does. :-)

If you're settled on your solution, and I haven't yet swayed your thinking, I won't try further to convince you. I'm happy to have a general solution to the problem.

[qx-bug-importer]

Thomas Herchenroeder (@thron7) wrote:

> If you're settled on your solution, and I haven't yet swayed your thinking, I > won't try further to convince you. I'm happy to have a general solution to the > problem.

I'm not settled, and willing to re-consider in better times. Just now, I guess this is as much as I can muster. The bug was not scheduled at all, and it was only a by-product of working on something else that I saw an opportunity to integrate the os environment. I did it on-the-fly, so to speak. I don't mind keeping it open, but it will hardly see any updates anytime soon, and will be un-targeted from 1.6 again. If we don't come across severe show-stoppers I guess we could have it in the release in the current form nevertheless, as experimental. Otherwise, I will have to remove it again.

[qx-bug-importer]

Tristan Koch (@trkoch) wrote:

Just to give my two cents… I like the feature, but I have my doubts that allowing the environment to overwrite the configuration is a good idea. First, I agree with Derrell that the implementation differs from what is typically expected.

To add another example, git (to my knowledge) reads in the order 1) environment 2) global (/etc) 3) user (~/.git) 4) project (PROJECT/.git)

Also…,

1) it may break existing setups.

Wrong environment variables (e.g. QOOXDOO_PATH) break all qooxdoo projects.

I added a QOOXDOO_PATH to my environment a while ago (thought it would work). The path changed in the meantime, but with the update it was read and therefore prevented the generator to find qooxdoo. It took me some time to connect the dots, because I was not thinking about the recent changes to the generator at first.

2) its inflexible.

What if you want to define a global default, but want to override it in certain projects? Say, you want to be on 'branch' most of the time, but for one project, try 'master'. Sure, you could prefix the generator with QOOXDOO_PATH but thats cumbersome.

3) no namespaces.

I can only guess know how many macros we have (documented and undocumented), but I would assume more than 10 at least? Some have a rather general name (ROOT, APPLICATION). Now, I could imagine some environments to have variables predefined that are macros in our context.

With the two-dollar convention, you could opt-in to read environment variables. It does not break existing setups, because it requires action from the user who wants to take advantage of the new feature. If you want to read the path from the environment, you map $QOOXDOO_PATH to $$QOOXDOO_PATH.

Maybe a compromise is to "whitelist" some environment variables?

[qx-bug-importer]

Thomas Herchenroeder (@thron7) wrote:

> 1) it may break existing setups. > > Wrong environment variables (e.g. QOOXDOO_PATH) break all qooxdoo projects.

Well, if you can influence generator macros through environment variables, you can also shoot yourself in the foot. As always.

> 2) its inflexible. > > What if you want to define a global default, but want to override it in certain > projects? Say, you want to be on 'branch' most of the time, but for one > project, try 'master'. Sure, you could prefix the generator with QOOXDOO_PATH > but thats cumbersome.

We had this argument before. You can edit your config.json's right away, without using environment variables at all. Or you can use different bash profiles for different projects.

> 3) no namespaces. > > I can only guess know how many macros we have (documented and undocumented), > but I would assume more than 10 at least? Some have a rather general name > (ROOT, APPLICATION). Now, I could imagine some environments to have variables > predefined that are macros in our context.

This is indeed an issue.

> > With the two-dollar convention, you could opt-in to read environment variables. > It does not break existing setups, because it requires action from the user who > wants to take advantage of the new feature. If you want to read the path from > the environment, you map $QOOXDOO_PATH to $$QOOXDOO_PATH.

Yes, but as I perceived it the most pressing issue (mostly voiced outside this bug) is being able to override QOOXDOO_PATH without modifying config.json. And as I wrote elsewhere, using a special convention for environment variables forces the programmer to commit to where a value will come from, rather than leaving it open for later. All your suggestions assume that it is easier to edit config.json then to modify the shell environment. But this is the exact opposite to how I e.g. understand Derrell.

> > Maybe a compromise is to "whitelist" some environment variables?

The minimal solution would probably be to restrict environment variable recognition to just QOOXDOO_PATH.

I will remove the current implementation until there is more clarity about the general direction this should take.

[qx-bug-importer]

Derrell Lipman (@derrell) wrote:

> Yes, but as I perceived it the most pressing issue (mostly voiced outside this > bug) is being able to override QOOXDOO_PATH without modifying config.json. And > as I wrote elsewhere, using a special convention for environment variables > forces the programmer to commit to where a value will come from, rather than > leaving it open for later. All your suggestions assume that it is easier to > edit config.json then to modify the shell environment. But this is the exact > opposite to how I e.g. understand Derrell.

Just to clarify, my preferred implementation would have a variable that is specified in config.json override what is in the environment. To accomplish my goal, using that implementation, I would expect that QOOXDOO_PATH would be removed entirely from config.json, so the the value in from the environment would be used. It certainly seems easier to tell everyone to set QOOXDOO_PATH in their environment, once, than to ask them to modify the config.json file of every application and contribution they ever use to match where they keep their framework.

Then, in those rare cases when I want to be able to use some different QOOXDOO_ PATH, I can either use your suggestion of an alias or shell command to specify a new location, or I could add, in that one app's config.json, an explicit QOOXDOO_PATH entry.

Derrell

ps. Actually, my statement about "preferred implementation" isn't quite true. My preferred implementation is one that actually exists, not one that gets remove now that it's finally here. :-)

[qx-bug-importer]

Thomas Herchenroeder (@thron7) wrote:

I removed os.environ except for QOOXDOO_PATH if it is set. Then the override semantics kick in. At least this is a focussed approach we can experiment with.

With 6306aee.

[qx-bug-importer]

Thomas Herchenroeder (@thron7) wrote:

> Just to clarify, my preferred implementation would have a variable that is specified in config.json override what is in the environment. To accomplish my goal, using that implementation, I would expect that QOOXDOO_PATH would be removed entirely from config.json, so the the value in from the environment would be used. It certainly seems easier to tell everyone to set QOOXDOOPATH in their environment, once, than to ask them to modify the config.json file of every application and contribution they ever use to match where they keep their framework. > > Then, in those rare cases when I want to be able to use some different QOOXDOO > PATH, I can either use your suggestion of an alias or shell command to specify a new location, or I could add, in that one app's config.json, an explicit QOOXDOO_PATH entry. >

I'm not so sure. What you say makes perfect sense in an "established" qooxdoo environment. One or a few apps, maybe some self-written libs, maybe some contribs. And then you upgrade your existing qooxdoo installation in a side-by-side fashion, so you're able to easily switch between the two. Then your approach is perfect.

I'm not sure, though, I want to impose on a casual or first-time user the force to set a mandatory environment key, before even a gui skeleton can be built. Think of the guys working on Windows. It's a journey even to get to the dialogue where you can set environment variables, and then you have to restart Cmd for it to take effect. It's clumsy. I would want to keep our zero-effort install as long as possible. But that means providing sensible defaults for all necessary generator macros, including QOOXDOO_PATH.

[qx-bug-importer]

Thomas Herchenroeder (@thron7) wrote:

The main use case, taking QOOXDOO_PATH from OS environment, is implemented. If we need support for more OS env vars this should be covered by fresh bugs, specific to the given env var. A general import of OS vars into the config name space seems questionable as this makes plenty of room for unintended name clashes and captures.

As I've written before I'm skeptical towards extra syntax for OS env vars in config.json ("$${FOO}" in Tristan's description), as it forces the presence of env vars and doesn't allow for in-file provision of the values.

[qx-bug-importer]

Thomas Herchenroeder (@thron7) wrote:

* BZ#3764 has been marked as a duplicate of this bug. *

[qx-bug-importer]

Martin Wittemann (@wittemann) wrote:

Closed all bugs already shipped with a release.