Bug: MSBuild forbids resetting of properties passed as arguments to any outer instance
I have noticed some odd behavior with MSBuild and properties that are specified on the command line or in MSBuild tasks. It is (sometimes?) not possible to change the value of such properties declaratively inside a <PropertyGroup> neither is it possible to change them using <CreateProperty>. In both cases the new value is *silently* discarded!
Worse, it appears that nested MSBuild instances also inherit the same problem. So if I happened to pass a property to an MSBuild script via the command line then use the <MSBuild> task to launch a nested instance of the interpreter, not only does the property get inherited into its scope when it shouldn't (because it was not specified in the Properties attribute) but I *still* can't override it! There's some ugly scope violation going on here within the build engine.
My typical workaround is to define a separate property inside the project with a different name than the one passed in from the outside. Then I assign it in a <PropertyGroup>.
I'll try to cook up a simple reproducible sample for this in a bit.
Hi Jeffrey
This is the designed behavior. /p switches win over anything. The argument was that the user should have the power to override anything in the build script to get what they want. I've heard arguments either way.
Can you describe what you're trying to achieve and can't get to happen with this design?
Dan
Basically I found this behavior confusing because:
1. I have yet to find any clear documentation about the effect.
2. There are no warnings or errors emitted when I attempt to modify the value of a property thus passed into a script. It should issue some kind of "read-only" error and point to an appropriate help topic like NAnt does.
3. It cost me a good 3 hours to figure this out! Why should it matter whether a property's value is assigned from outside by passing it into the script or from inside over the course of the evaluation? This rule introduces a kind of dynamic scope behavior. The value of the property ends up immutably bound by name because of how the MSBuild script or one of its calling scripts was invoked! So now I need to worry about name clashes that may occur between properties used in a top-level script and those used in one several levels down.
I can make my desired behavior happen but I have to take special steps to ensure I never try to write to a property whose value may be specified from outside the script. For instance, a common pattern is to verify that a passed in property has a trailing slash and to add it if absent. This code fails silently because the property is effectively immutable.
I'd argue it's okay for the properties to be considered immutable in the newly spawned script. Some warning or error should be issued when attempting to modify them. On the other hand, I believe the value of those properties should not flow downwards into called scripts unless they are explicitly passed in.
I can't think of any case where I would want to deliberately override the behavior of a script by forcing a property to evaluate to some specified value. I would take that as a sign of a poorly designed script. It should be redesigned to handle the desired use case. Most probably the "default value" idiom is in order: add a check to see if the property value is empty before setting it to some new value. That enables calling scripts to override default behavior safely without any of this other nonsense.
Cheers,
Jeff.
Jeff,
Personally I agree with you about /p properties winning. If the targets author is careful, they'll put the "check for empty" conditions on all possible property definitions, so it isn't necessary for /p properties to always win. If the targets author isn't careful, well a property that always wins could cause assumptions to in the targets file to be incorrect and cause any kind of problems. It's pretty dangerous, when the original scenario was just to do something like /p:Configuration=Debug;OutDir=c:\stuff which doesn't need any special override powers. On balance I think the design choice was wrong. I may be unaware of some other consideration that influenced the original design. Also we have to bear in mind that some users don't have the power to change the targets file, or possibly even the project, they just want the build to do what they want, no niceties.
I disagree that /p properties should not flow down into child targets. Imagine I want to build a whole tree of projects in Debug configuration -- /p:Configuration=Debug really needs to flow to achieve this in a reasonable way. You still have the power to truncate the flow by passing a new property on the <MSBuild> task's Properties parameter. Without the immutability, this is the same behavior as environment variables.
Neither behavior can be changed as many people will be depending on it. It's possible we could add a warning when a targets file tries to modify a global (ie, immutable) property. If you like you could open a bug (see instructions at the top of the forum). Since some existing build processes probably are designed to expect this it will be noise for some people so I'd suspect a message instead would be better. In fact, depending on how people are hosting MSBuild, a new warning could actually be escalated to a build failure: and we don't have a toggle to disable MSBuild warnings yet.
Not if and until typing/scoping are one day added would we have a chance to have another think about this behavior. At that point for example I can see adding a Strict="true" attribute to <Project> that meant amongst other things that /p properties were just like regular property tags at the very top of the script.
As always if you or any other customers have ideas for how we can make MSBuild better, we'd love to hear them and let's discuss them here. Two things to bear in mind: you'll understand, as a platform we must have to have a very high bar for changes that would break any existing users, whether we like the existing behavior or not; and we have tried to have a bias that I'd like to keep for keeping the number of MSBuild concepts small.
Dan
This posting provided 'as-is' with no warranties.
I am not sure why you are not seeing CreateProperty unable to modify the values passed in from the command line - you should be able to do that, and we just verified that it works.
I do hear you on the fact that we don't warn when /p takes over any property definitions that are already in the project file. We will certainly think about warning in that case. Please take some time to open a bug against us - we certainly can open a bug on our end, but opening a bug yourself would allow us to interact regarding the progress of the bug as we make progress on the issue.
Thanks.
Faisal Mohamood | Program Manager | Visual Studio - MSBuild
Not sure about CreateProperty anymore. Could be I was mistaken as I was unable to reproduce the behavior. Hrm.
Anyways, I created a bug for it:
https://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=219016
Thanks for your time!
Jeff.
