Skip to contentSkip to conversation

Making exported variables really global

PATCH In review Andreas Vogel 3 participants 9 messages Mar 4, 2012

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Making exported variables really global

Hi all,

This tiny patch makes exported variables really global. Right now exported variables are set in the context of a submenu when running it. But any changes to those vars are lost when leaving the submenu. This patch sets and exports all vars in the calling context for the submenu which are exported in the context of the submenu.

This feature is needed in order to have submenus implementing something like setting boot options like boot params, language setttings and the like using environment variables.

Any comments? If it's OK any chance to be included?

Andreas

=== modified file 'grub-core/normal/context.c'

normal/context.c +14 -0
@@ -126,6 +126,20 @@
 
       for (p = grub_current_context->vars[i]; p; p = q)
 	{
+	  if (p->global)
+	    {
+	      /* Set and export all global variables inside
+		 the calling/previous context.  */
+	      struct grub_env_context *tmp_context = grub_current_context;
+	      grub_current_context = grub_current_context->prev;
+	      if (grub_env_set (p->name, p->value) == GRUB_ERR_NONE)
+		{
+		  grub_env_export (p->name);
+		  grub_register_variable_hook (p->name, p->read_hook, p->write_hook);
+		}
+	      grub_current_context = tmp_context;
+	    }
+
 	  q = p->next;
           grub_free (p->name);
 	  grub_free (p->value);
#1 lore raw
Quoted 7 lines · Andreas Vogel

Hi all,

This tiny patch makes exported variables really global. Right now exported variables are set in the context of a submenu when running it. But any changes to those vars are lost when leaving the submenu. This patch sets and exports all vars in the calling context for the submenu which are exported in the context of the submenu.

We follow the bash behaviour. And the bash behaviour is exactly what GRUB does:

[email protected]:14:15:11:~/grub2/bzr/mainline$ bash
[email protected]:14:15:15:~/grub2/bzr/mainline$ export XYZ=x
[email protected]:14:15:19:~/grub2/bzr/mainline$ exit
[email protected]:14:15:20:~/grub2/bzr/mainline$ echo $XYZ

[email protected]:14:15:24:~/grub2/bzr/mainline$

Signature · 2 lines
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
#2 lore raw

Am 04.03.2012 14:18, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:

Quoted 8 lines · Andreas Vogel
Quoted 7 lines

Hi all,

This tiny patch makes exported variables really global. Right now exported variables are set in the context of a submenu when running it. But any changes to those vars are lost when leaving the submenu. This patch sets and exports all vars in the calling context for the submenu which are exported in the context of the submenu.

Quoted 9 lines

We follow the bash behaviour. And the bash behaviour is exactly what GRUB does:

[email protected]:14:15:11:~/grub2/bzr/mainline$ bash
[email protected]:14:15:15:~/grub2/bzr/mainline$ export XYZ=x
[email protected]:14:15:19:~/grub2/bzr/mainline$ exit
[email protected]:14:15:20:~/grub2/bzr/mainline$ echo $XYZ

[email protected]:14:15:24:~/grub2/bzr/mainline$

I understand your point, but IMHO we do need to be able to set global vars in submenus, that's my patch about.

Otherwise how can you use a "Settings..." submenu to configure environment variables which are used by other menu entries for booting? The analogy with bash is not useful here.

#3 lore raw
Quoted 16 lines · Andreas Vogel

Am 04.03.2012 14:18, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:

Quoted 15 lines · Andreas Vogel
Quoted 7 lines

Hi all,

This tiny patch makes exported variables really global. Right now exported variables are set in the context of a submenu when running it. But any changes to those vars are lost when leaving the submenu. This patch sets and exports all vars in the calling context for the submenu which are exported in the context of the submenu.

We follow the bash behaviour. And the bash behaviour is exactly what GRUB does:

[email protected]:14:15:11:~/grub2/bzr/mainline$ bash
[email protected]:14:15:15:~/grub2/bzr/mainline$ export XYZ=x
[email protected]:14:15:19:~/grub2/bzr/mainline$ exit
[email protected]:14:15:20:~/grub2/bzr/mainline$ echo $XYZ
Quoted 8 lines
Quoted 2 lines

[email protected]:14:15:24:~/grub2/bzr/mainline$

I understand your point, but IMHO we do need to be able to set global vars in submenus, that's my patch about.

Otherwise how can you use a "Settings..." submenu to configure environment variables which are used by other menu entries for booting? The analogy with bash is not useful here.

We can add a command with traits of both "menuentry" (same context, same menu) and "submenu" (new context, new menu), sth like submemu_source (same context, new menu)

Quoted 1 line
Signature · 2 lines
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
#4 lore raw

Am 04.03.2012 15:10, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:

Quoted 23 lines · Andreas Vogel
Quoted 22 lines

Am 04.03.2012 14:18, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:

Quoted 9 lines · Andreas Vogel
Quoted 8 lines

Hi all,

This tiny patch makes exported variables really global. Right now exported variables are set in the context of a submenu when running it.
But any changes to those vars are lost when leaving the submenu. This patch sets and exports all vars in the calling context for the submenu which are exported in the context of the submenu.

Quoted 9 lines

We follow the bash behaviour. And the bash behaviour is exactly what GRUB does:

[email protected]:14:15:11:~/grub2/bzr/mainline$ bash
[email protected]:14:15:15:~/grub2/bzr/mainline$ export XYZ=x
[email protected]:14:15:19:~/grub2/bzr/mainline$ exit
[email protected]:14:15:20:~/grub2/bzr/mainline$ echo $XYZ

[email protected]:14:15:24:~/grub2/bzr/mainline$

I understand your point, but IMHO we do need to be able to set global vars in submenus, that's my patch about.

Quoted 6 lines
Quoted 3 lines

Otherwise how can you use a "Settings..." submenu to configure environment variables which are used by other menu entries for booting? The analogy with bash is not useful here.

We can add a command with traits of both "menuentry" (same context, same menu) and "submenu" (new context, new menu), sth like submemu_source (same context, new menu)

Yes, that's one possible solution.

Why establishing a new command instead of using an option for submenu, let's say "--export 0|1"?

#5 lore raw

Am 04.03.2012 17:16, schrieb Andreas Vogel:

Quoted 27 lines

Am 04.03.2012 15:10, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:

Quoted 26 lines · Andreas Vogel
Quoted 25 lines

Am 04.03.2012 14:18, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:

Quoted 9 lines · Andreas Vogel
Quoted 8 lines

Hi all,

This tiny patch makes exported variables really global. Right now exported variables are set in the context of a submenu when running it.
But any changes to those vars are lost when leaving the submenu. This patch sets and exports all vars in the calling context for the submenu which are exported in the context of the submenu.

Quoted 9 lines

We follow the bash behaviour. And the bash behaviour is exactly what GRUB does:

[email protected]:14:15:11:~/grub2/bzr/mainline$ bash
[email protected]:14:15:15:~/grub2/bzr/mainline$ export XYZ=x
[email protected]:14:15:19:~/grub2/bzr/mainline$ exit
[email protected]:14:15:20:~/grub2/bzr/mainline$ echo $XYZ

[email protected]:14:15:24:~/grub2/bzr/mainline$

I understand your point, but IMHO we do need to be able to set global vars in submenus, that's my patch about.

Otherwise how can you use a "Settings..." submenu to configure environment variables which are used by other menu entries for booting? The analogy with bash is not useful here.

Quoted 7 lines
Quoted 3 lines

We can add a command with traits of both "menuentry" (same context, same menu) and "submenu" (new context, new menu), sth like submemu_source (same context, new menu)

Yes, that's one possible solution.

Why establishing a new command instead of using an option for submenu, let's say "--export 0|1"?

Please let me summarize this issue after thinking about it again:

Right now the submenu command opens a new context. All exported vars in the parent context are also exported to the new context of the submenu. This is the same behaviour like for Unix shells. The menuentry command doesn't open a new context, it runs in the context of the caller.

For submenus there are 3 possibilities for how to behave:

1) Create a new context and run the submenu in the new context. After exiting the submenu, all variables in the submenu context are lost (this is the current behavior).

2) Don't create a new context and run the submenu in the same context as the caller. This would be the same behavior as for menuentry commands. This can be implemented by having a separate submenu command (e.g. submenu_source, or submenu_shared) or introducing a new submenu option (e.g. --shared 0| 1).

3) Create a new context for the submenu. On exit all exported environment variables in the submenu context are exported to the context of the caller. By this you have the advantage that all local (unexported) variables in the submenu context are really local, even for further submenu entries in the submenu, but all exported variables are global for the whole menu tree. Like for option 2) this can be achieved by establishing a new command or using an new option for the submenu command. But this behavior can also be the default behavior for submenus without the need to be able to enable/disable it.

As I'm quite unfamiliar with the internal design of GRUB, I'm not sure what option is most adequate, even though I'd vote for option 3).

I cannot see the point why it's so important to run a submenu in its own shell (= context) and why menuentries of a submenu shouldn't be able to access the same environment variables like the menuentries of a parent menu. In my opinion the analogy to shells and sub-shells in Unix isn't really adequate because for me a menu tree is an entity on its own, regardless of being implemented as commands (like in GRUB) or using some kind of configuration file defining the menu tree.
On the other side i see some advantages for submenus having their own context, e.g. to have local environment variables which don't 'pollute' the calling context. That's the reason why I vote for option 3) with the addition that it's not

Please excuse my lengthy exploration about this issue, but it's a very important point for GRUB. Not being able to set globally defined environment variables (= exported toplevel context vars) in submenus prevented until now having customization submenus and doing a search in I-Net about this issue, I've found a lot of posts desperately asking for this feature.

As you can see from my patch in the OP, the modification is quite simple and from what i can see the implementation of the other options presented above are not much more complicated.

I'm willing to implement and supply whatever will be decided in the hope that we can get it into the upcoming version. We just need to get a final decision. So any comments are, as always, highly welcomed.

Andreas

#6 lore raw

Am 04.03.2012 18:55, schrieb Andreas Vogel:

Quoted 7 lines

Please let me summarize this issue after thinking about it again:

Right now the submenu command opens a new context. All exported vars in the parent context are also exported to the new context of the submenu. This is the same behaviour like for Unix shells. The menuentry command doesn't open a new context, it runs in the context of the caller.

Quoted 5 lines

For submenus there are 3 possibilities for how to behave:

1) Create a new context and run the submenu in the new context. After exiting the submenu, all variables in the submenu context are lost (this is the current behavior).

Personally, I like that solution best.
What's the problem? Currently there's no way and absolutely no way to reliably get data from a submenu to a parent menu. This makes it impossible to use submenus e.g. for settings menus.
By sharing the context this would not only be consistent with menuentries but while significantly improving the situation it can't cause any major harm. In case somebody relies on variables being local, I'm pretty sure there's always a solution to achieve similar behaviour by multiple variables. So making the context shared wouldn't create any real troubles. But the way it is now - separate context for submenu - does create problems. For example no settings menus and there's no solution for that need at all! As in my opinion this is a major applications for submenus something should be done about it.

Quoted 5 lines

2) Don't create a new context and run the submenu in the same context as the caller. This would be the same behavior as for menuentry commands. This can be implemented by having a separate submenu command (e.g. submenu_source, or submenu_shared) or introducing a new submenu option (e.g. --shared 0| 1).

While I could live with such a solution, it seems just flawed to me. Such a construct introduces unecessary complexity and inconsistency. If there's such a switch for submenus why shouldn't there be one for source, menuentry, ...?
As pointed out previously a separated context offers pretty much nothing that's not possible with a shared one, so in most cases --shared 1 would be fine. But having to write every time --shared 1 after a submenu or submenu_shared is quite cluttered. The use of such constructs doesn't create the impression of a well designed language for my part either.

Quoted 9 lines

3) Create a new context for the submenu. On exit all exported environment variables in the submenu context are exported to the context of the caller. By this you have the advantage that all local (unexported) variables in the submenu context are really local, even for further submenu entries in the submenu, but all exported variables are global for the whole menu tree. Like for option 2) this can be achieved by establishing a new command or using an new option for the submenu command. But this behavior can also be the default behavior for submenus without the need to be able to enable/disable it.

I don't really like this one. It introduces a completely new and complicated behaviour, which is not commonly found among other software, especially not with bash.
I see the risk that even with very good documentation, this implementation leads to unexpected results. And I think it could happen that users won't be able to understand what's causing the problems because shared and local variables are mixed. Such unintuitive solutions should be avoided wherever possible and there are two other possibilities which are in my view much better.

Anyway, just some thoughts from a user.

Andreas

#7 lore raw

First of all thanks for your detailed comment.

Am 04.03.2012 22:03, schrieb Andreas Born:

Quoted 13 lines

Am 04.03.2012 18:55, schrieb Andreas Vogel:

Quoted 7 lines

Please let me summarize this issue after thinking about it again:

Right now the submenu command opens a new context. All exported vars in the parent context are also exported to the new context of the submenu. This is the same behaviour like for Unix shells. The menuentry command doesn't open a new context, it runs in the context of the caller.

Quoted 5 lines

For submenus there are 3 possibilities for how to behave:

1) Create a new context and run the submenu in the new context. After exiting the submenu, all variables in the submenu context are lost (this is the current behavior).

Quoted 13 lines

Personally, I like that solution best.
What's the problem? Currently there's no way and absolutely no way to reliably get data from a submenu to a parent menu. This makes it impossible to use submenus e.g. for settings menus.
By sharing the context this would not only be consistent with menuentries but while significantly improving the situation it can't cause any major harm. In case somebody relies on variables being local, I'm pretty sure there's always a solution to achieve similar behaviour by multiple variables. So making the context shared wouldn't create any real troubles. But the way it is now - separate context for submenu - does create problems. For example no settings menus and there's no solution for that need at all! As in my opinion this is a major applications for submenus something should be done about it.

It seems you are missing the point here. Option 1) describes the situation like it is right now and means not to change the code and not being able to have submenus which are able to set variables in the callers context. As far as I understood Vladimirs first answer, there is already some kind of common understanding that this is something what should be changed in the future.

Quoted 5 lines
Quoted 5 lines

2) Don't create a new context and run the submenu in the same context as the caller. This would be the same behavior as for menuentry commands. This can be implemented by having a separate submenu command (e.g. submenu_source, or submenu_shared) or introducing a new submenu option (e.g. --shared 0| 1).

Quoted 10 lines

While I could live with such a solution, it seems just flawed to me. Such a construct introduces unecessary complexity and inconsistency. If there's such a switch for submenus why shouldn't there be one for source, menuentry, ...?
As pointed out previously a separated context offers pretty much nothing that's not possible with a shared one, so in most cases --shared 1 would be fine. But having to write every time --shared 1 after a submenu or submenu_shared is quite cluttered. The use of such constructs doesn't create the impression of a well designed language for my part either.

As far as I understood your comment, you're voting for option 2) but without needing to specify that the context should be shared with the calling context. In other words sharing the context should be enabled by default. Right?

Quoted 9 lines
Quoted 9 lines

3) Create a new context for the submenu. On exit all exported environment variables in the submenu context are exported to the context of the caller. By this you have the advantage that all local (unexported) variables in the submenu context are really local, even for further submenu entries in the submenu, but all exported variables are global for the whole menu tree. Like for option 2) this can be achieved by establishing a new command or using an new option for the submenu command. But this behavior can also be the default behavior for submenus without the need to be able to enable/disable it.

Quoted 9 lines

I don't really like this one. It introduces a completely new and complicated behaviour, which is not commonly found among other software, especially not with bash.
I see the risk that even with very good documentation, this implementation leads to unexpected results. And I think it could happen that users won't be able to understand what's causing the problems because shared and local variables are mixed. Such unintuitive solutions should be avoided wherever possible and there are two other possibilities which are in my view much better.

The rule for this option is very simple and can be expressed in one
sentence: "Regarding the menu system, all exported variables have a global scope."
BTW, right now GRUB makes a difference between local (unexported) and global (exported) vars: global vars are "handed over" to the context of submenus, local vars not. Options 3) just enhances this paradigm and treats global vars as read/write instead of readonly.
The comparison with bash is senseless - we are talking about a menu system and not about subshells in general.

Andreas

#8 lore raw

Am 04.03.2012 23:48, schrieb Andreas Vogel:

Quoted 51 lines

First of all thanks for your detailed comment.

Am 04.03.2012 22:03, schrieb Andreas Born:

Quoted 13 lines

Am 04.03.2012 18:55, schrieb Andreas Vogel:

Quoted 7 lines

Please let me summarize this issue after thinking about it again:

Right now the submenu command opens a new context. All exported vars in the parent context are also exported to the new context of the submenu. This is the same behaviour like for Unix shells. The menuentry command doesn't open a new context, it runs in the context of the caller.

Quoted 5 lines

For submenus there are 3 possibilities for how to behave:

1) Create a new context and run the submenu in the new context. After exiting the submenu, all variables in the submenu context are lost (this is the current behavior).

Quoted 13 lines

Personally, I like that solution best.
What's the problem? Currently there's no way and absolutely no way to reliably get data from a submenu to a parent menu. This makes it impossible to use submenus e.g. for settings menus.
By sharing the context this would not only be consistent with menuentries but while significantly improving the situation it can't cause any major harm. In case somebody relies on variables being local, I'm pretty sure there's always a solution to achieve similar behaviour by multiple variables. So making the context shared wouldn't create any real troubles. But the way it is now - separate context for submenu - does create problems. For example no settings menus and there's no solution for that need at all! As in my opinion this is a major applications for submenus something should be done about it.

It seems you are missing the point here. Option 1) describes the situation like it is right now and means not to change the code and not being able to have submenus which are able to set variables in the callers context. As far as I understood Vladimirs first answer, there is already some kind of common understanding that this is something what should be changed in the future.

Quoted 5 lines
Quoted 5 lines

2) Don't create a new context and run the submenu in the same context as the caller. This would be the same behavior as for menuentry commands. This can be implemented by having a separate submenu command (e.g. submenu_source, or submenu_shared) or introducing a new submenu option (e.g. --shared 0| 1).

Quoted 10 lines

While I could live with such a solution, it seems just flawed to me. Such a construct introduces unecessary complexity and inconsistency. If there's such a switch for submenus why shouldn't there be one for source, menuentry, ...?
As pointed out previously a separated context offers pretty much nothing that's not possible with a shared one, so in most cases --shared 1 would be fine. But having to write every time --shared 1 after a submenu or submenu_shared is quite cluttered. The use of such constructs doesn't create the impression of a well designed language for my part either.

Quoted 4 lines

As far as I understood your comment, you're voting for option 2) but without needing to specify that the context should be shared with the calling context. In other words sharing the context should be enabled by default. Right?

Yes. Somehow missed that 1) wasn't about this. Sorry. Let's call it 4), which would be: no new context is created with submenus.

Quoted 19 lines
Quoted 10 lines
Quoted 10 lines

3) Create a new context for the submenu. On exit all exported environment variables in the submenu context are exported to the context of the caller. By this you have the advantage that all local (unexported) variables in the submenu context are really local, even for further submenu entries in the submenu, but all exported variables are global for the whole menu tree. Like for option 2) this can be achieved by establishing a new command or using an new option for the submenu command. But this behavior can also be the default behavior for submenus without the need to be able to enable/disable it.

Quoted 9 lines

I don't really like this one. It introduces a completely new and complicated behaviour, which is not commonly found among other software, especially not with bash.
I see the risk that even with very good documentation, this implementation leads to unexpected results. And I think it could happen that users won't be able to understand what's causing the problems because shared and local variables are mixed. Such unintuitive solutions should be avoided wherever possible and there are two other possibilities which are in my view much better.

Quoted 3 lines

The rule for this option is very simple and can be expressed in one
sentence: "Regarding the menu system, all exported variables have a global scope."

Yes, but no matter where and how prominent you put it who reads this?
And not introducing the complexity of contexts with subshells, which offer little benefit here, makes it even easier.

Quoted 3 lines

BTW, right now GRUB makes a difference between local (unexported) and global (exported) vars: global vars are "handed over" to the context of submenus, local vars not.

That's the same distinction subshells make so it's a common and well-known concept.

Quoted 2 lines

Options 3) just enhances this paradigm and
treats global vars as read/write instead of readonly.

We actually then have three cases (there are 2 with 4) and even with 2) ):
1) local variables: only visible in the current context
2) exported/global variables: copied to new contexts
3) really global variables: copied back and forth between contexts
This is unnecessarily complicated and using 3) only with a special command like submenu doesn't make the distinction easier to grasp. In my view this is even easier:
"In the menu system there is one shared context."
And this behaves like menuentries.

Quoted 2 lines

The comparison with bash is senseless - we are talking about a menu system and not about subshells in general.

Bash and it's concepts are well known. On the other hand introducing new, not commonly used concepts makes it unnecessarily hard to use and understand the grub shell. Why make it complicated if you can keep it simple?

Andreas