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

Custom build options #450

Closed
gfraiteur opened this issue Apr 13, 2015 · 9 comments
Closed

Custom build options #450

gfraiteur opened this issue Apr 13, 2015 · 9 comments

Comments

@gfraiteur
Copy link

How can custom compiler extensions (ICompilerModule or IProjectCompiler) consume project options, so these extensions can consume custom options?

Thank you.

@gfraiteur
Copy link
Author

I'm confirming that custom build options would be a requirement to properly integrate PostSharp with ASP.NET v5.

What we need to match is the MSBuild ability to pass custom properties from the command line. We would need a similar mechanism to pass custom build options or custom properties to the project/application context.

The workaround currently would be to pass the properties through environment variables, but this is not as convenient.

Custom properties could be passed using a command-line argument like --property:Name=Value.

Custom properties could be exposed as a service to be consumed from IServiceProvider, for instance:

public interface ICustomPropertyRepository
{
  string GetPropertyValue(string name);
}

Would that be an acceptable pull request?

@davidfowl
Copy link
Member

So potential problems:

  • Name spacing properties? MSBuild is pretty terrible in that everything is global and everyone can see everything by default. How do I target a specific set feature (like postsharp)? Do we prefix? PostSharp:Key=Value. We might look into using IConfiguration here.
  • How does this work for runtime compilation?

I'm not ready to accept pull requests that change that much of the pipeline yet. We're actually in the middle of a major rewrite/refactor of the runtime.

@gfraiteur
Copy link
Author

Namespace: I would let that problem to conventions. For instance the property name could be dotted.

Runtime compilation: That would work the same. We just want to pass temporary options through the command line. But it means there should be a way to pass permanent options to project.json, which makes things more complex.

@gfraiteur
Copy link
Author

Another problem: the scope of project.json is the project, but the scope of command-line options are the "solution". But it does not really differ on how MSBuild properties work, I think. The question is whether the command line takes priority over project.json.

Some use cases for custom options in PostSharp:

  • Attach debugger to PostSharp (typically a temporary option)
  • Disable or escalate PostSharp warnings (typically permanent)
  • Code optimization level (permanent, depends on configuration)
  • Define custom properties consumed by custom PostSharp aspects

@gfraiteur
Copy link
Author

I would like to know if the feature would be accepted as a pull request and what would be the design approval process (because I would like to get approval before investing more time).

Thanks.

@gfraiteur
Copy link
Author

After analysis, here is my proposal:

End user's perspective

  1. New command-line option "-p|--property", for instance -p:dotted.property.name=value (property assigned to a value) or -p:dotted.property.name (property assigned with a null value).
  2. In project.json, new section properties next to the compilerOptions section. That is, the properties section can appear both at root level, at configuration level, and at framework level. Example:
{
    "version": "1.0.0-*",
    "compilationOptions": { "allowUnsafe": true, "warningsAsErrors": true },
    "properties": { "postsharp.ignoredWarnings" : "PS1234" },
    "dependencies": {
        "PostSharp": "6.0.6",
    },
    "configurations": {
        "Debug": {
            "compilationOptions": { "optimize": false },
            "properties": { "postsharp.emitRuntimeValidation": "true" }
        },
        "Release": {
            "compilationOptions": { "optimize": true },
            "properties": { "postsharp.emitRuntimeValidation": "true" }
        },

    }
}
  1. The global.json file would be augmented with the properties section.
  2. All properties are strings. Two rationales: don't expose Json outside of the parser assembly, and make properties fully compatible with the command line.
  3. Precedence: from lower to higher: command-line argument, global.json, root-level project.json, configuration, framework. Note that the order is compatible with the one of compilerOptions.

ICompilerModule's perspective

  1. Interface IProjectContext has a new property ICompilerOptions CompilerOptions { get;}.
  2. Interface ICompilerOptions has a new property IDictionary<string,string> Properties { get; }.

Exposing properties on ICompilerOptions has the benefit of minimizing the number of changes in the API. Also, some compiler modules will re-use compiler options. For instance, PostSharp could interpret the optimize compiler property.

DNX perspective

  1. Implement properties in CompilerOptions and ProjectContext.
  2. Add merging of properties in CompilerOptions.Combine.
  3. Implement changes in project.json in the Project class.
  4. Implement changes in global.json in the GlobalSettings class.
  5. Add proper testing (didn't figure out this point yet).

Open questions

  1. Should we design a system to prevent a property to be overriden, sor instance "properties" : { "my.property" : { "sealed": true, "value: "myValue"} }.
  2. Is properties a good enough name? Does it convey the fact that it is a custom property, for consumption by "extensions"?

I'm looking forward to reading your feedback to know if I can move further with this design.

@gfraiteur
Copy link
Author

@davidfowl Any comment on this proposal? Such as "awesome", "no way", "wait after //build/", "bother my colleague X" ...

@davidfowl
Copy link
Member

@gfraiteur This is promising

/cc @victorhurdugaci

@davidfowl
Copy link
Member

Msbuild is back 😉

@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants