Property Expansion for Environment Variables [message #637039] |
Wed, 03 November 2010 17:51  |
Eclipse User |
|
|
|
Hi,
I'm currently working on fixing
http://issues.hudson-ci.org/browse/HUDSON-7978 for the Hudson plugin.
I want to avoid to write ALL environment variables as JVM properties in
the command line because that would get huge. However, all necessary
values are set as environment variables of the buckminster process.
Is there currently a way to have properties replaced by environment
variables?
If not, what's your opinion in including envVars into the property
expansion?
Best regards,
Johannes
|
|
|
|
|
|
|
Re: Property Expansion for Environment Variables [message #637570 is a reply to message #637525] |
Sat, 06 November 2010 19:53   |
Eclipse User |
|
|
|
Hi Thomas,
please see comments below
Am 06.11.2010 08:20, schrieb Thomas Hallgren:
> Hi Johannes,
>
> There are two ways of resolving this. Either you add all properties to
> the property sets that you want to use like you suggest, or you let the
> ExpandingProperties recognize the ${env_var: prefix}. The advantage with
> the former is that it is simple and that the environment is included
> when iterating over the keys in the map. The advantage of the latter is
> that it will keep the map small (there's some copying going on) and is
> in line with current Eclipse functionality. My vote is on the latter
> because the use of env_var is uncommon (you're the first one asking for
> it the four years Buckminster has been active ;-) ) and we might have an
> urge to enable more prefixes.
>
> Perhaps we should even consider having the ExpandingProperties consult
> the Eclipse property expander when we encounter a prefix? That way all
> the eclipse specific properties would work. What do you think?
>
> - thomas
>
What you say makes a lot of sense to me. Also consulting the Eclipse
property expander makes it very easy for users to contribute completely
new kinds of (dynamic) properties by creating an extension for
org.eclipse.core.variables.(dynamic/static)Variables and therefore
participating with very little effort in the buckminster property
expansion process.
The env_var prefix is contributed by org.eclipse.debug.core which is
part of a regular buckminster installation anyway, so no additional
bundles would be required for the desired functionality.
So far so good, but there's a few things I'm a little uncertain about:
1. API consistency
ExpandingProperties is often regarded as a simple Map from what I can
see in the source. Consulting the StringVariableManager during the
expansion but ignoring it during the other API methods might behave a
little inconsistent for API consumers. E.g.
"keySet() doesn't return my env_var:FOOBAR but 'expand' replaces the
value just fine"
Including the new variable scope in all methods might on the other hand
negate the advantage of keeping the map small.
2. The implementation
Using the StringVariableManager seems straight forward enough:
VariablesPlugin.getDefault().getStringVariableManager().perf ormStringSubstitution(expression,
false);
But ExpandingProperties is one of the core classes of Buckminster. It's
widely used and it seems a rather sophisticated implementation to me.
However, there don't seem to be any real unit tests in place for this
and I'm somewhat concerned of what I could break if include the
StringVariableManager there. Especially if you prefer to not only
include it in the property expansion, but keep the API consistent with
the new property scopes (because that'd mean to adjust quite a few of
the methods at once).
To wrap it up:
If you think env_vars and custom vars are corner cases and the Eclipse
mechanism should only be consulted during the actual expansion but
ignored in all other methodes, then I'd assume my changes should be made
around here in ExpandingProperties#checkedExpand :
Object propVal = (props instanceof ExpandingProperties<?>) ?
((ExpandingProperties<?>) props).getExpandedProperty(propKey,
recursionGuard + 1) : props.get(propKey);
But if you'd prefer that the other methods also include the additional
variable scopes for sake of consistency, then I'm afraid I'll need some
advice on how to apply the changes without breaking any of the classes
that depend on ExpandingProperties :-/
Best regards,
Johannes
|
|
|
|
|
Powered by
FUDForum. Page generated in 0.03206 seconds