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

Assign-once properties #1673

Open
rainersigwald opened this issue Feb 6, 2017 · 5 comments
Open

Assign-once properties #1673

rainersigwald opened this issue Feb 6, 2017 · 5 comments
Labels
Area: Language Issues impacting the MSBuild programming language. triaged

Comments

@rainersigwald
Copy link
Member

Some properties define things that should logically be constant throughout a project, but MSBuild doesn't provide any way to enforce that they're not modified after they've been read.

Take BaseIntermediateOutputPath, for example. Since it's a foundational property, it's read early in SDK .props to set other properties. But that means that it can't just be set in the user's .csproj, like most properties--because that's after decisions have already been made according to its earlier value.

If there was a way to specify that any write to a given property after it had been read was an authoring error, this could be much easier to chase down.

The error shouldn't fire for things that are attempting to set defaults like

<P Condition=" '$(P)' == '' ">DefaultValue</P>
@cdmihai
Copy link
Contributor

cdmihai commented Feb 7, 2017

To avoid dealing with the <P Condition=" '$(P)' == '' ">DefaultValue</P> idiom, would it make sense to have an entire property group as set-once?. That way, you can put all the logic for defining a property in a set-once property group. All subsequent attempts to write to that property after the first set-once occurrence would result in an error

<PropertyGroup SetOnce="true">
   <P Condition=" '$(TFM)' == '1'">foo</P>
   <P Condition=" '$(TFM)' == '2'">bar</P>
   <P Condition=" '$(P)' == ''">tar</P>
   <P2>foo</P2>
</PropertyGroup>

<!--this results in error-->
<PropertyGroup>
   <P2>foo</P2>
</PropertyGroup>

<!--this results in error-->
<PropertyGroup SetOnce="true">
   <P Condition=" '$(p)' == '' and '$(TFM)' == '3'">zar</P>
</PropertyGroup>

Or is this too restrictive?

Another issue is the scope of the set-once constraint, should the set-once constraint for a property survive msbuild task invocations or not?

@am11
Copy link
Member

am11 commented Feb 7, 2017

@rainersigwald, in addition to this, are there any plans to provide an explicit idiom to consumers for accomplishing its reciprocal:

override this foundational property and re-evaluate all dependents

at certain stage of the build? (setting the value of OutputPath from result of user-defined Inline Tasks for example)

I couldn't have figured out without @jeffkl's help how to override OutputPath and IntermediateOutputPath from an inline-task: jeffkl/MSBuild-NetCore#2.

@rainersigwald
Copy link
Member Author

Originally posted by @kingces95 at #848.

I ran across a bug where I unintentionally defined a property with the same name as another property defined in a target file I included. It would be nice if msbuild had facilities to help detect this type of issue. For instance, if I could have somehow marked my property immutable then msbuild could have issued an error when the property was subsequently overwritten in the imported target file.

One possible way to mark properties as immutable would be via an IsReadOnly attribute applied to property and propertyGroup elements. Also, it would also be nice to be able to change the default to readonly via, possibly, a ReadOnlyProperties attribute applied on the Project element.

There is already a mechanism by which command line properties are treated as immutable. Maybe that same logic could be used to enforce a new readonly property feature. That logic would have to be extended to issue an error instead of silently failing after attempting to update the property as is currently the case for command line properties. I'd like to see an error in that case as well. Possibly enabled via an environment variable ala MSBuildWarnOnUninitializedProperty but MSBuildWarnOnWriteToCommandLineProperty.

Warm regards,
Chris

@rainersigwald
Copy link
Member Author

For my set-defaults concern, there's already code that does that using CurrentlyEvaluatingPropertyElementName. This could piggy-back off that.

@rainersigwald
Copy link
Member Author

@am11 I don't see a way to do that without a massive change to the MSBuild evaluation model which doesn't seem likely. If you'd like to see it, open another issue and we can discuss why it's hard.

@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Language Issues impacting the MSBuild programming language. triaged
Projects
None yet
Development

No branches or pull requests

4 participants